Fix problem with deleting temporary folders on Windows #460

eblis · 2022-09-02T11:38:30Z

This is another attempt for pull request python-poetry/poetry#5171

Added tests for changed code.
Updated documentation for changed code.

From what I can tell the relevant code has been moved from poetry to poetry-core, I've created a new pull request targeting this repository.

Resolves: python-poetry/poetry#1031

…tch #1032)

sonarcloud · 2022-09-02T11:50:57Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

da1910 · 2022-10-10T12:46:51Z

This issue still exists (albeit intermittently) and is again blocking our CI process, is there anything we as users can do to get this fix expedited?

Andy-Grigg · 2022-10-11T13:20:25Z

I'm seeing this issue as well. From what I can tell, this fix will resolve the issue and stop our intermittent Windows build failures.

HenriqueAJNB · 2023-01-04T17:47:45Z

Hi, I'm waiting for this PR to merge. Can anyone give us some feedback on how is it going? How can we help to make it happen?

DudeNr33 · 2023-01-05T13:31:23Z

I would also really appreciate this fix, as I am once again sitting in front of my computer, restarting poetry commands over and over for simply adding a new dependency.

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

Workarounds: - Flaky Python temporary directory removal on Windows <python-poetry/poetry-core#460>. - Mandatory timeout <nick-fields/retry#107>. - Misleading `poetry --directory` functionality <https://github.com/python-poetry/poetry/issues/7363>/Missing option for working directory <nick-fields/retry#89>.

src/poetry/core/utils/helpers.py

for more information, see https://pre-commit.ci

dimbleby · 2023-04-26T16:54:58Z

python/cpython#24793 adds ignore_cleanup_errors to the parameters of tempfile.TemporaryDirectory(), from python 3.10 onwards - which seems to be aimed at exactly this.

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

Or perhaps - given how longstanding this must be (python-poetry/poetry#1031) - poetry should simply use TemporaryDirectory always, with ignore_cleanup_errors=True where available. Windows users on older pythons must be an ever-smaller minority.

Reworked test for robust_rmtree to actually delete the temp stuff created during the test.

da1910 · 2023-04-27T09:10:24Z

python/cpython#24793 adds ignore_cleanup_errors to the parameters of tempfile.TemporaryDirectory(), from python 3.10 onwards - which seems to be aimed at exactly this.

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

Or perhaps - given how longstanding this must be (python-poetry/poetry#1031) - poetry should simply use TemporaryDirectory always, with ignore_cleanup_errors=True where available. Windows users on older pythons must be an ever-smaller minority.

I would strongly favour making some effort to fix this for python 3.8 and 3.9, the fact that it's taken this long to address isn't really a reason to wash our hands of support, Python 3.9 is supported for another two and a half years after all!

dimbleby · 2023-04-27T11:40:20Z

Python 3.9 is supported for another two and a half years after all!

Sure. However, the current situation has been tolerated for all of that and then some; and for the huge majority who are not using older Pythons on Windows, I'd have a much higher degree of confidence in the standard library's temporary-directory implementation than in a home-grown implementation .

So I do think it would be preferable in general to use TemporaryDirectory(). I don't have very strong opinions about whether Windows with python 3.8 and 3.9 are important; but IMO it would at least be not absurd to choose to keep poetry code simple and decline to fix this for them.

da1910 · 2023-04-27T11:53:22Z

Python 3.9 is supported for another two and a half years after all!

Sure. However, the current situation has been tolerated for all of that and then some; and for the huge majority who are not using older Pythons on Windows, I'd have a much higher degree of confidence in the standard library's temporary-directory implementation than in a home-grown implementation .

So I do think it would be preferable in general to use TemporaryDirectory(). I don't have very strong opinions about whether Windows with python 3.8 and 3.9 are important; but IMO it would at least be not absurd to choose to keep poetry code simple and decline to fix this for them.

I would agree about simplicity for certain, but I'm going to disagree with you about the scope of the problem. I've personally wasted several days of my life rerunning CI because of this issue, and there are plenty of other people on the original thread who have had the same experience for the last four years (I've only been using poetry 'in anger' for the last year or so).

Windows may be a minority for open-source developers, but for casual python users it's vastly the most common OS, and to have a project like poetry deprioritise fixes for windows specific bugs with in-support python versions seems slightly unfriendly.

Secrus · 2023-04-27T12:33:35Z

So perhaps the implementation of temporary_directory() should go: "if python >= 3.10 then use TemporaryDirectory(ignore_cleanup_errors=True), else do this much more complicated thing"

This is the solution I would go for and which is the most likely going to be accepted.

dimbleby · 2023-04-27T14:14:03Z

"unfriendly" isn't the aim, more like "pragmatic"!

Windows makes up less than 3% of poetry's downloads according to https://pypistats.org/packages/poetry - bottom of page. This is surely a very imperfect measure but, so far as I know, the best we have.

I don't know how to split this up by python version, and I certainly don't know how to split this up by "open-source developers" vs "casual users". Either way I suppose it's true that 3% of poetry's users today is a larger absolute number than 3% of poetry's users was in 2019.

Anyway: if maintainers prefer to take on the additional complication, I've no fight to pick with that.

… directory.

for more information, see https://pre-commit.ci

dimbleby · 2023-04-27T14:48:39Z

re the typing errors in the pipeline: so far as I can see no-one ever passes any arguments to temporary_directory(), so I'd just remove the args and kwargs altogether.

…ectory is actually used.

eblis · 2023-04-27T15:27:43Z

re the typing errors in the pipeline: so far as I can see no-one ever passes any arguments to temporary_directory(), so I'd just remove the args and kwargs altogether.

I saw this message too late, spent a bit of time fighting with mypy and in the end just added the argument to kwargs before calling the function, seemed to please mypy.

…her way around

xmnlab · 2023-04-27T15:41:36Z

Windows makes up less than 3% of poetry's downloads according to https://pypistats.org/packages/poetry

but maybe this analysis about this number could be a bit biased ... because poetry would be used more by devs and on CI ...
the experience programming on windows is not that great ... so probably most python dev doesn't use windows (IMHO) ... and if we it is not working on CI using windows, no one is testing their packages with poetry for windows. (that is my case)

so I guess that this number makes sense. but maybe if the poetry works well for windows .. that number will increase (at least a bit)

xmnlab · 2023-04-27T15:45:48Z

and just as an extra info, on conda/conda-forge the downloads for windows is almost the same number for osx (for the latest version there):
https://anaconda.org/conda-forge/poetry/files

radoering

I think we now have a compromise that each side can live with.

Just some minor nitpicks regarding the test before merging it.

tests/utils/test_helpers.py

for more information, see https://pre-commit.ci

sonarcloud · 2023-04-30T11:47:29Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.4.2` -> `1.5.0` | --- ### Release Notes <details> <summary>python-poetry/poetry</summary> ### [`v1.5.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#150---2023-05-19) [Compare Source](python-poetry/poetry@1.4.2...1.5.0) ##### Added - **Introduce the new source priorities `explicit` and `supplemental`** ([#7658](python-poetry/poetry#7658), [#6879](python-poetry/poetry#6879)). - **Introduce the option to configure the priority of the implicit PyPI source** ([#7801](python-poetry/poetry#7801)). - Add handling for corrupt cache files ([#7453](python-poetry/poetry#7453)). - Improve caching of URL and git dependencies ([#7693](python-poetry/poetry#7693), [#7473](python-poetry/poetry#7473)). - Add option to skip installing directory dependencies ([#6845](python-poetry/poetry#6845), [#7923](python-poetry/poetry#7923)). - Add `--executable` option to `poetry env info` ([#7547](python-poetry/poetry#7547)). - Add `--top-level` option to `poetry show` ([#7415](python-poetry/poetry#7415)). - Add `--lock` option to `poetry remove` ([#7917](python-poetry/poetry#7917)). - Add experimental `POETRY_REQUESTS_TIMEOUT` option ([#7081](python-poetry/poetry#7081)). - Improve performance of wheel inspection by avoiding unnecessary file copy operations ([#7916](python-poetry/poetry#7916)). ##### Changed - **Remove the old deprecated installer and the corresponding setting `experimental.new-installer`** ([#7356](python-poetry/poetry#7356)). - **Introduce `priority` key for sources and deprecate flags `default` and `secondary`** ([#7658](python-poetry/poetry#7658)). - Deprecate `poetry run <entry point>` if the entry point was not previously installed via `poetry install` ([#7606](python-poetry/poetry#7606)). - Only write the lock file if the installation succeeds ([#7498](python-poetry/poetry#7498)). - Do not write the unused package category into the lock file ([#7637](python-poetry/poetry#7637)). ##### Fixed - Fix an issue where Poetry's internal pyproject.toml continually grows larger with empty lines ([#7705](python-poetry/poetry#7705)). - Fix an issue where Poetry crashes due to corrupt cache files ([#7453](python-poetry/poetry#7453)). - Fix an issue where the `Retry-After` in HTTP responses was not respected and retries were handled inconsistently ([#7072](python-poetry/poetry#7072)). - Fix an issue where Poetry silently ignored invalid groups ([#7529](python-poetry/poetry#7529)). - Fix an issue where Poetry does not find a compatible Python version if not given explicitly ([#7771](python-poetry/poetry#7771)). - Fix an issue where the `direct_url.json` of an editable install from a git dependency was invalid ([#7473](python-poetry/poetry#7473)). - Fix an issue where error messages from build backends were not decoded correctly ([#7781](python-poetry/poetry#7781)). - Fix an infinite loop when adding certain dependencies ([#7405](python-poetry/poetry#7405)). - Fix an issue where pre-commit hooks skip pyproject.toml files in subdirectories ([#7239](python-poetry/poetry#7239)). - Fix an issue where pre-commit hooks do not use the expected Python version ([#6989](python-poetry/poetry#6989)). - Fix an issue where an unclear error message is printed if the project name is the same as one of its dependencies ([#7757](python-poetry/poetry#7757)). - Fix an issue where `poetry install` returns a zero exit status even though the build script failed ([#7812](python-poetry/poetry#7812)). - Fix an issue where an existing `.venv` was not used if `in-project` was not set ([#7792](python-poetry/poetry#7792)). - Fix an issue where multiple extras passed to `poetry add` were not parsed correctly ([#7836](python-poetry/poetry#7836)). - Fix an issue where `poetry shell` did not send a newline to `fish` ([#7884](python-poetry/poetry#7884)). - Fix an issue where `poetry update --lock` printed operations that were not executed ([#7915](python-poetry/poetry#7915)). - Fix an issue where `poetry add --lock` did perform a full update of all dependencies ([#7920](python-poetry/poetry#7920)). - Fix an issue where `poetry shell` did not work with `nushell` ([#7919](python-poetry/poetry#7919)). - Fix an issue where subprocess calls failed on Python 3.7 ([#7932](python-poetry/poetry#7932)). - Fix an issue where keyring was called even though the password was stored in an environment variable ([#7928](python-poetry/poetry#7928)). ##### Docs - Add information about what to use instead of `--dev` ([#7647](python-poetry/poetry#7647)). - Promote semantic versioning less aggressively ([#7517](python-poetry/poetry#7517)). - Explain Poetry's own versioning scheme in the FAQ ([#7517](python-poetry/poetry#7517)). - Update documentation for configuration with environment variables ([#6711](python-poetry/poetry#6711)). - Add details how to disable the virtualenv prompt ([#7874](python-poetry/poetry#7874)). - Improve documentation on whether to commit `poetry.lock` ([#7506](python-poetry/poetry#7506)). - Improve documentation of `virtualenv.create` ([#7608](python-poetry/poetry#7608)). ##### poetry-core ([`1.6.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.6.0)) - Improve error message for invalid markers ([#569](python-poetry/poetry-core#569)). - Increase robustness when deleting temporary directories on Windows ([#460](python-poetry/poetry-core#460)). - Replace `tomlkit` with `tomli`, which changes the interface of some *internal* classes ([#483](python-poetry/poetry-core#483)). - Deprecate `Package.category` ([#561](python-poetry/poetry-core#561)). - Fix a performance regression in marker handling ([#568](python-poetry/poetry-core#568)). - Fix an issue where wildcard version constraints were not handled correctly ([#402](python-poetry/poetry-core#402)). - Fix an issue where `poetry build` created duplicate Python classifiers if they were specified manually ([#578](python-poetry/poetry-core#578)). - Fix an issue where local versions where not handled correctly ([#579](python-poetry/poetry-core#579)). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/717 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

eblis added 3 commits September 2, 2022 14:34

Fix problem with deleting temporary folders on Windows (re-created pa…

4083ab0

…tch #1032)

Attempt to fix flake8 error

523fb72

Added return type to test function

dfc8bf0

da1910 mentioned this pull request Oct 17, 2022

poetry add/update/install can fail on Windows during temp directory cleanup python-poetry/poetry#1031

Closed

l0b0 mentioned this pull request Jan 18, 2023

test: Poetry on Windows linz/emergency-management-tools#120

Merged

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 18, 2023

test: Poetry on Windows

a87ba1e

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 18, 2023

test: Poetry on Windows

e391f59

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 18, 2023

test: Poetry on Windows

5def11e

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 18, 2023

test: Poetry on Windows

9f7eecd

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 20, 2023

test: Poetry on Windows

992ce59

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

finswimmer requested a review from a team January 20, 2023 08:20

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 29, 2023

test: Poetry on Windows

58fd986

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

l0b0 added a commit to linz/emergency-management-tools that referenced this pull request Jan 30, 2023

test: Poetry on Windows

4d67c50

Works around flaky temporary directory removal on Windows <python-poetry/poetry-core#460>.

cdgriffith reviewed Feb 1, 2023

View reviewed changes

src/poetry/core/utils/helpers.py Outdated Show resolved Hide resolved

eblis and others added 6 commits April 24, 2023 17:34

Removed safe_rmtree() and included the logic in robust_rmtree instead.

5707d05

Merge remote-tracking branch 'upstream/main'

617e0c9

Fixed import errors in test_helpers.py after fixing merge conflicts.

3464eb1

[pre-commit.ci] auto fixes from pre-commit.com hooks

09a14a1

for more information, see https://pre-commit.ci

Import MockerFixture under a type check block.

de39b18

[pre-commit.ci] auto fixes from pre-commit.com hooks

8334d72

for more information, see https://pre-commit.ci

eblis requested review from cdgriffith and da1910 April 24, 2023 14:51

Merge branch 'main' into main

80a836a

Accept both str and Path in robust_rmtree.

ba70189

Reworked test for robust_rmtree to actually delete the temp stuff created during the test.

eblis and others added 2 commits April 27, 2023 17:28

For Python 3.10+ use tempfile.TemporaryDirectory() to get a temporary…

f1db06b

… directory.

[pre-commit.ci] auto fixes from pre-commit.com hooks

add8534

for more information, see https://pre-commit.ci

Added unit tests for python 3.9- and 3.10+ to check that TemporaryDir…

8cf4deb

…ectory is actually used.

Mixed the names for the temporary_directory() tests, should be the ot…

ab38ee9

…her way around

eblis requested a review from radoering April 27, 2023 16:34

dimbleby mentioned this pull request Apr 28, 2023

Poetry self update fails on Windows with Access Denied, leaving poetry in a broken state python-poetry/poetry#7610

Closed

4 tasks

radoering requested changes Apr 28, 2023

View reviewed changes

tests/utils/test_helpers.py Outdated Show resolved Hide resolved

tests/utils/test_helpers.py Show resolved Hide resolved

eblis and others added 2 commits April 29, 2023 10:02

Code review suggestions

0e16074

[pre-commit.ci] auto fixes from pre-commit.com hooks

a2d6dcf

for more information, see https://pre-commit.ci

eblis requested a review from radoering April 29, 2023 07:03

Merge branch 'main' into main

99ef81e

radoering approved these changes Apr 30, 2023

View reviewed changes

radoering merged commit f314393 into python-poetry:main Apr 30, 2023

radoering mentioned this pull request May 12, 2023

release: bump version to 1.6.0 #584

Merged

dimbleby mentioned this pull request May 14, 2023

prefer stdlib TemporaryDirectory python-poetry/poetry#7680

Draft

radoering mentioned this pull request May 18, 2023

release: bump version to 1.5.0 python-poetry/poetry#7947

Merged

saviit mentioned this pull request Oct 9, 2023

TIM setup voi epäonnistua Poetryn bugiin Python riippuvuuksia asennettaessa Windowsilla TIM-JYU/TIM#3493

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix problem with deleting temporary folders on Windows #460

Fix problem with deleting temporary folders on Windows #460

eblis commented Sep 2, 2022 •

edited by finswimmer

Loading

sonarcloud bot commented Sep 2, 2022

da1910 commented Oct 10, 2022

Andy-Grigg commented Oct 11, 2022

HenriqueAJNB commented Jan 4, 2023 •

edited

Loading

DudeNr33 commented Jan 5, 2023

dimbleby commented Apr 26, 2023 •

edited

Loading

da1910 commented Apr 27, 2023

dimbleby commented Apr 27, 2023

da1910 commented Apr 27, 2023

Secrus commented Apr 27, 2023

dimbleby commented Apr 27, 2023

dimbleby commented Apr 27, 2023

eblis commented Apr 27, 2023

xmnlab commented Apr 27, 2023

xmnlab commented Apr 27, 2023

radoering left a comment

sonarcloud bot commented Apr 30, 2023

Fix problem with deleting temporary folders on Windows #460

Fix problem with deleting temporary folders on Windows #460

Conversation

eblis commented Sep 2, 2022 • edited by finswimmer Loading

sonarcloud bot commented Sep 2, 2022

da1910 commented Oct 10, 2022

Andy-Grigg commented Oct 11, 2022

HenriqueAJNB commented Jan 4, 2023 • edited Loading

DudeNr33 commented Jan 5, 2023

dimbleby commented Apr 26, 2023 • edited Loading

da1910 commented Apr 27, 2023

dimbleby commented Apr 27, 2023

da1910 commented Apr 27, 2023

Secrus commented Apr 27, 2023

dimbleby commented Apr 27, 2023

dimbleby commented Apr 27, 2023

eblis commented Apr 27, 2023

xmnlab commented Apr 27, 2023

xmnlab commented Apr 27, 2023

radoering left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Apr 30, 2023

eblis commented Sep 2, 2022 •

edited by finswimmer

Loading

HenriqueAJNB commented Jan 4, 2023 •

edited

Loading

dimbleby commented Apr 26, 2023 •

edited

Loading